-
Notifications
You must be signed in to change notification settings - Fork 237
Integrate Automated QDQ placement tool - Part 2 #702
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Integrate Automated QDQ placement tool - Part 2 #702
Conversation
|
Hi @ajrasane , could you help me review this PR, thanks! |
3f7ff31 to
d3a6765
Compare
616285d to
c95939a
Compare
| quantized_tensors = set() | ||
|
|
||
| for node in onnx_model.graph.node: | ||
| if node.op_type == "QuantizeLinear": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If --dq_only is enabled, there may only be the DQ node indicating that a tensor is being quantized. Please verify that those cases are supporting with this function.
See
| "--dq_only", |
4468ca2 to
bc87ca7
Compare
| from modelopt.onnx.quantization.graph_utils import get_tensor_consumer_node_indices | ||
|
|
||
| # Module logger | ||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use the logger created here for all the logging?
https://github.com/NVIDIA/Model-Optimizer/blob/727da95a9188aaeef6872a61acae9f1ffae844f6/modelopt/onnx/logging_config.py
| divergent_outputs = [ | ||
| out.name for out in node.outputs if self._is_tensor_divergent(out.name) | ||
| ] | ||
| is_divergent = len(divergent_outputs) > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to:
is_divergent = any(self._is_tensor_divergent(out.name) for out in node.outputs)| for next_node_idx in self.tensor_users_map[output.name]: | ||
| if next_node_idx not in reachable: | ||
| reachable[next_node_idx] = distance + 1 | ||
| queue.append((next_node_idx, distance + 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we skip adding the nodes to the queue if the distance + 1 < maxsteps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think no need to add this extra check. They will be skipped at Line 285 when they are poped.
if distance >= max_steps:
continue
| 2. All nodes between divergence and convergence | ||
|
|
||
| **Algorithm:** | ||
| 1. Identify all branches from the divergent node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a mandatory criteria that a region must start with a divergent node and end with a convergent node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- a region must start with a divergent node
Yes, when linear probe reaches a divergent node, RegionSeach always tries to create a new region. - a region must end with a convergent node
If the convergent node is too far (>= 10 steps), RegionSeach will treatcurrent divergent nodeas orphane, and tries to probe from its output branches.
|
|
||
| # Share the tensor users map from Phase 1 to avoid recomputation. | ||
| # This map is expensive to build and is shared across all refinements. | ||
| region_builder.tensor_users_map = region_partitioner.tensor_users_map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also share the forward_reachable_nodes map form Phase 1 to avoid recomputation?
Signed-off-by: Will Guo <[email protected]>
bc87ca7 to
6f809d7
Compare
What does this PR do?
Type of change: new feature
Overview: This PR integrate automated Q/DQ placement tool to ModelOpt. This PR is 2/4 parts of the cahnges.
Part 1: #701
Part 2: #702
Part 3: #703
Part 4: #704
This PR contains the following changes:
Usage
Example output:
Testing
Implemented unit tests for new classes. All unit tests could get pass locally.
Before your PR is "Ready for review"
Additional Information